-
Notifications
You must be signed in to change notification settings - Fork 87
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Pass compilation context via swift info in hmap creation due to rules_swift 2.1.1 #906
Conversation
) | ||
|
||
providers = [ | ||
DefaultInfo( | ||
files = depset([headermap]), | ||
), | ||
apple_common.new_objc_provider(), | ||
cc_info_provider, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is cc_info_provider still necessary for rules_swift 1.18.0?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1.18 def. need it
not sure about 2.1.1, so far it seems that the logic depends on it if it is not a swift file to compile?
based on the commit i think it is apple_common.new_objc_provider()
we will remove soon as latest bazel don't have Objc provider under apple_common anymore: https://bazel.build/rules/lib/toplevel/apple_common (7.3 still has it)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we stack this on #903 just to confirm CI is all green before merging both
changed base to luis/add-clang-attr-to-merged-swift-info-for-framework and will merge that branch once the PR is all green |
ac323d2
into
luis/add-clang-attr-to-merged-swift-info-for-framework
…s_swift 2.1.1 (#907) same idea as #906 this time is for framework rule. The CI error trying to fix in this PR is: ``` # Execution platform: @local_config_platform//:host error: generate-pch command failed with exit code 1 (use -v to see invocation) tests/ios/unit-test/test-imports-app/TestImports-App-Bridging-Header.h:1:9: error: 'TestImports-App/Header2.h' file not found #import <TestImports-App/Header2.h> ^ 1 error generated. <unknown>:0: error: failed to emit precompiled header 'bazel-out/ios-sim_arm64-min12.0-applebin_ios-ios_sim_arm64-dbg-ST-c2aefc9133a8/bin/_pch_output_dir/TestImports-App-Bridging-Header-swift_28IU2BV1DK30K-clang_1FNSWCOAS4SUQ.pch' for bridging header 'tests/ios/unit-test/test-imports-app/TestImports-App-Bridging-Header.h' 2 errors generated. error: fatalError ``` This time this header is brought in by the framework rule The idea here is to create the `clang_module` based of the same compilation context as CcInfo and construct the final swift info based on it. With this no need to differentiate based on if VFS turned on or not
One of the CI error is:
This is because this header was suppose to be an input to Swift compile via hmap creation rule. But in this breaking change bazelbuild/rules_swift@d68b214 rules_swift stopped collecting compilation context from
CcInfo
if the target is Swift code (line 1555 ofswift/internal/compiling.bzl
). Instead it depends on reading the same info fromclang_module.compilation_context
which is available fromSwiftInfo
provider (SwiftInfo -> modules -> clang)This PR deals with header maps side where we propagate compilation context via this
clang
moduleNext step is to do the same for other places where we need to pass the compilation context
CI for this PR should no longer produce this error (but will still fail on some other tests)